Skip to content

fix(format): preserve model column dialect - #5926

Open
fresioAS wants to merge 3 commits into
SQLMesh:mainfrom
fresioAS:fix_columns_dialect
Open

fix(format): preserve model column dialect#5926
fresioAS wants to merge 3 commits into
SQLMesh:mainfrom
fresioAS:fix_columns_dialect

Conversation

@fresioAS

Copy link
Copy Markdown
Contributor

Description

Summary

  • Fixes regression introduced in fix(format): keep MODEL/AUDIT/METRIC header dialect-agnostic (#5773) #5864
  • Keep SQLMesh MODEL metadata dialect-agnostic during formatting.
  • Render the MODEL columns schema using the model's dialect.
  • Preserve dialect-specific column types such as DATETIME2(6) for T-SQL and Fabric.
  • Keep generic metadata values such as descriptions and booleans unchanged.

Context

MODEL headers were recently changed to use SQLGlot's generic generator so that
T-SQL would not rewrite SQLMesh boolean properties such as FALSE into
(1 = 0).

However, the columns property contains dialect-specific data types. Rendering
it with the generic generator changed DATETIME2(6) into TIMESTAMP(6) for
T-SQL and Fabric models.

This change keeps the generic header behavior while rendering only the
columns schema with the model dialect.

Test Plan

  • Added regression coverage for both tsql and fabric.
  • Verified descriptions remain string literals.
  • Verified formatting booleans remain FALSE.
  • Full tests/core/test_dialect.py suite passes: 161 tests.
  • Ruff and mypy pass.

Checklist

  • I have run make style and fixed any issues
  • I have added tests for my changes (if applicable)
  • All existing tests pass (make fast-test)
  • My commits are signed off (git commit -s) per the DCO

fresioAS and others added 2 commits July 28, 2026 15:06
@mday-io

mday-io commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

@fresioAS

Sharing some findings from digging into this area. They support the approach here and suggest a way to extend it.

  1. columns isn't the only property that lost its dialect. Rendering the whole header generically affects every property whose value is user-authored SQL. On main today, a T-SQL model with audits (my_audit(threshold := CAST('2024-01-01' AS DATETIME2))) formats to '2024-01-01'::TIMESTAMP, the same silent type downgrade as columns, just unreported so far. The same applies to expression values in physical_properties, and to time_data_type nested inside the SCD kinds. So the columns-only fix leaves some corruption in place.

  2. Probably don't need a hand-maintained property list. ModelMeta already declares which properties hold expressions (columns, audits, signals, partitioned_by, physical_properties, …) versus which are SQLMesh's own scalars (allow_partials, description, kind, …). Deriving the policy from those annotations classifies all of them, and the failure mode inverts usefully: a property that's missed doesn't get canonicalized, rather than having a user's SQL silently rewritten.

I prototyped this as a one-condition change to the hook in this PR. Swapping the columns name check for a policy lookup - extended to AUDIT/METRIC headers and nested kinds. It fixes the audits/physical_properties/time_data_type cases too, and sqlmesh format --transpile starts converting column types correctly again while keeping booleans safe. No regressions found with this approach.

Happy to open it as a follow-up on top of this, or hand it over if you'd rather fold it in. This PR's diagnosis and hook design are what it's built on.

One process note independent of the approach: whatever lands is worth calling out as a behavior change in the release notes. This would be the third consecutive release that reformats T-SQL headers

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants